Allow parallel execution of NAS backup and delete commands by jmsperu · Pull Request #12847 · apache/cloudstack

jmsperu · 2026-03-17T19:34:13Z

Summary

Change executeInSequence() from true to false in TakeBackupCommand and DeleteBackupCommand
Allows the KVM agent to process multiple backup/delete operations concurrently via its existing worker thread pool
RestoreBackupCommand and PrepareForBackupRestorationCommand remain sequential (they modify VM state)

Motivation

Currently all backup commands are serialized on the agent — a large VM backup (e.g. 100+ GB taking 2+ hours) blocks all other backup and delete operations on the same host. This is the root cause of backup schedule delays and timeouts in environments with many VMs per host.

Each backup operation:

Mounts its own temporary NFS directory (mktemp -d -t csbackup.XXXXX)
Operates on independent VM disks via separate QEMU block jobs
Has no shared state with other backup operations

There is no technical reason to serialize them. The agent already has a thread pool (requestHandler) that can execute multiple commands concurrently — this change simply allows backup commands to use it.

Impact

Hosts with 10+ VMs will see significantly faster backup completion (backups run in parallel instead of queuing)
NFS bandwidth is shared across concurrent backups (can be controlled with the bandwidth throttle flag from PR nasbackup.sh: add bandwidth throttle via -b flag #12846)
No change to management server scheduling — it already submits backups as independent async jobs

Test plan

Schedule 3+ VM backups at the same time on one host — verify they run concurrently (check virsh domjobinfo on multiple VMs)
Verify each backup gets its own mount point (no mount conflicts)
Run backup + delete concurrently — verify no interference
Verify restore operations still execute sequentially
Monitor host I/O during concurrent backups — consider using -b bandwidth throttle if NFS saturates

Change executeInSequence() to return false for TakeBackupCommand and DeleteBackupCommand, allowing the KVM agent to process multiple backup/delete operations concurrently via its worker thread pool. Previously, all backup commands were serialized — a large VM backup (e.g. 100+ GB taking 2+ hours) would block all other backup and delete operations on the same host. Since each backup mounts its own temporary NFS directory and operates on independent VM disks, there is no shared state requiring serialization. Restore and PrepareForBackupRestoration commands remain sequential as they modify VM state that should not be concurrent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-03-18T04:50:56Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 16.26%. Comparing base (61afb4c) to head (b2cd0bf).
⚠️ Report is 18 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...rg/apache/cloudstack/backup/TakeBackupCommand.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #12847      +/-   ##
============================================
+ Coverage     16.24%   16.26%   +0.01%     
- Complexity    13411    13432      +21     
============================================
  Files          5664     5665       +1     
  Lines        500463   500572     +109     
  Branches      60779    60792      +13     
============================================
+ Hits          81308    81420     +112     
+ Misses       410059   410046      -13     
- Partials       9096     9106      +10

Flag	Coverage Δ
uitests	`4.15% <ø> (ø)`
unittests	`17.11% <0.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR changes CloudStack backup agent command scheduling by making the NAS backup take and delete commands eligible for parallel execution on the agent (instead of being forced through the single-threaded “in-sequence” queue).

Changes:

Make TakeBackupCommand.executeInSequence() return false.
Make DeleteBackupCommand.executeInSequence() return false.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java	Marks backup creation command as non-sequential (parallelizable) on the agent.
core/src/main/java/org/apache/cloudstack/backup/DeleteBackupCommand.java	Marks backup deletion command as non-sequential (parallelizable) on the agent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

    @Override
    public boolean executeInSequence() {
-        return true;
+        return false;


abh1sar · 2026-03-24T04:07:07Z

    @Override
    public boolean executeInSequence() {
-        return true;
+        return false;


This is a valid concern. I think TakeBackup command can still be parallelized but not DeleteBackup. @jmsperu

@jmsperu will you be making this change?

winterhazel · 2026-05-13T02:06:54Z

@JoaoJandre could you have a look at whether this change would affect KBOSS?

JoaoJandre · 2026-05-14T11:56:53Z

@JoaoJandre could you have a look at whether this change would affect KBOSS?

@winterhazel KBOSS uses its own commands to send messages to the agent, thus this does not affect it.
In any case, KBOSS commands have this flag as false as well.

winterhazel · 2026-05-21T14:19:12Z

@jmsperu could you have a look at @abh1sar's comment at #12847 (comment)?

jmsperu · 2026-05-22T21:38:09Z

Thanks for the ping @winterhazel — and @abh1sar's point at #12847 (comment) is well taken. DeleteBackup needs sequential execution because parallel deletes against the same NAS path can race on the cleanup of the chain (we've also seen the rm -rf vs mount/unmount interleave produce stuck mounts). Will restrict the parallelization to TakeBackup only in the next push — DeleteBackup keeps its current sequential behaviour. Thanks @JoaoJandre for the KBOSS confirmation too.

@abh1sar

Revert DeleteBackupCommand to executeInSequence()=true per @abh1sar's review. Backup creation is safe to parallelize across VMs because each VM writes to its own NAS path. Backup deletion is not safe to parallelize the same way: incremental chains share parent qcow2 files on the NAS, and concurrent deletions of sibling or descendant nodes can race on the underlying file references and the chain metadata. Until that's modeled explicitly, keep DeleteBackup serialized. TakeBackup parallelization stands.

jmsperu · 2026-05-22T21:52:52Z

Pushed b2cd0bf — reverted the DeleteBackupCommand change so only TakeBackupCommand flips to executeInSequence() = false. Delete stays serialized as you suggested @abh1sar (incremental chains share parent qcow2 files on the NAS, concurrent sibling/descendant deletes can race on shared file refs).

winterhazel · 2026-05-22T21:54:02Z

@blueorangutan package

blueorangutan · 2026-05-22T21:56:04Z

@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

jmsperu · 2026-05-22T22:38:47Z

Notes on the current red checks — all three appear unrelated to this change (which is a single executeInSequence() boolean flip on TakeBackupCommand and touches no other code path):

codecov/project — reports 4.15% (-12.10%) project-coverage delta. That number is noise: the patch surface is one line so the codecov math is degenerate. codecov/patch (the one that actually reflects this PR) passes.
build (component/test_persistent_networks ...) — failure is test_vpc_force_delete throwing builtins.Exception plus test_admin_project_creation going EXCEPTION on one retry while the other retry passed. Known flake pattern; VPC teardown / project-creation tests are not on any code path reachable from a backup executeInSequence flip.
build (component/test_lb_secondary_ip ...) — failure is test_03_delete_instance in test_memory_limits. Memory-limits resource-accounting tests are timing-sensitive on the simulator and have no overlap with backup orchestration.

Happy to rebase to retrigger if a reviewer prefers a fresh CI run, but the change itself is unaffected by these matrices. Other 14 checks green, 8 still running.

blueorangutan · 2026-05-22T22:48:47Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17997

sureshanaparti requested review from abh1sar and weizhouapache March 18, 2026 04:43

sureshanaparti added status:needs-testing component:backup component:kvm labels Mar 18, 2026

sureshanaparti requested a review from Copilot March 18, 2026 05:06

Copilot started reviewing on behalf of sureshanaparti March 18, 2026 05:06 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

DaanHoogland added this to the 4.20.4 milestone Mar 19, 2026

winterhazel requested a review from JoaoJandre May 13, 2026 02:06

Conversation

jmsperu commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Impact

Test plan

Uh oh!

codecov Bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

abh1sar Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

abh1sar Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

winterhazel commented May 13, 2026

Uh oh!

JoaoJandre commented May 14, 2026

Uh oh!

winterhazel commented May 21, 2026

Uh oh!

jmsperu commented May 22, 2026

Uh oh!

jmsperu commented May 22, 2026

Uh oh!

winterhazel commented May 22, 2026

Uh oh!

blueorangutan commented May 22, 2026

Uh oh!

jmsperu commented May 22, 2026

Uh oh!

blueorangutan commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jmsperu commented Mar 17, 2026 •

edited

Loading

codecov Bot commented Mar 18, 2026 •

edited

Loading